feat: add file deletion feature to menu actions#78
Conversation
Implements file deletion functionality with confirmation dialog. Users can now delete Claude configuration files directly from the interactive menu using the 'x' key shortcut. Changes: - Add delete file action to menu with 'x' key binding - Require confirmation before deleting files - Update README with new keyboard shortcut - Add tests for delete functionality - Add TypeScript as dev dependency for build compatibility Resolves user request to incorporate file deletion feature to cleanup redundant tools/agents alongside existing edit/view options.
WalkthroughA new "Delete file" action is added to the application menu with confirmation workflow. The action is documented in README, implemented in the menu hook with fs/promises unlink operation, tested with comprehensive test cases, and TypeScript dependency is updated. Changes
Sequence DiagramsequenceDiagram
actor User
participant Menu
participant Handler
participant Confirmation
participant fs as fs/promises
User->>Menu: Select Delete File (x)
Menu->>Handler: Trigger delete action
Handler->>Handler: Load fs/promises
Handler->>Confirmation: Set confirmation message
Handler->>Confirmation: Enable confirmation UI
Handler->>Handler: Queue performDelete<br/>Return empty string
Confirmation->>User: Show confirmation dialog
alt User Confirms
User->>Confirmation: Confirm deletion
Confirmation->>fs: unlink(file path)
fs->>fs: Delete file
fs-->>Handler: Success
Handler-->>User: Display success message
else User Cancels
User->>Confirmation: Cancel
Confirmation-->>User: Deletion cancelled
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/FileList/MenuActions/hooks/useMenu.ts (2)
238-241: Enhance error handling for common deletion failure scenarios.The generic error handling may not provide clear guidance for common failure cases like permission errors or attempting to delete an already-deleted file.
Consider adding specific error handling:
const performDelete = async () => { - await fs.unlink(file.path); + try { + await fs.unlink(file.path); + } catch (error) { + if ( + error && + typeof error === 'object' && + 'code' in error + ) { + if (error.code === 'ENOENT') { + throw new Error('File no longer exists (may have been deleted externally)'); + } + if (error.code === 'EPERM' || error.code === 'EACCES') { + throw new Error('Permission denied: Cannot delete this file'); + } + } + throw error; + } return `✅ File deleted: ${basename(file.path)}`; };
244-246: Consider shorter confirmation message for better UX.The confirmation message is quite long. A more concise version would improve readability while maintaining clarity.
setConfirmMessage( - `Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`, + `Delete "${basename(file.path)}"? This cannot be undone.`, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)package.json(1 hunks)src/components/FileList/MenuActions.test.tsx(5 hunks)src/components/FileList/MenuActions/hooks/useMenu.ts(1 hunks)
🔇 Additional comments (3)
README.md (1)
179-179: LGTM!The documentation clearly describes the new delete functionality and appropriately notes that confirmation is required. The placement at the end of the keyboard shortcuts list is logical for a destructive action.
src/components/FileList/MenuActions.test.tsx (1)
57-57: Test coverage for Delete File display and ordering looks good.The tests properly verify that the Delete File action appears in the UI with the correct label, key binding, and ordering (after Open File).
Also applies to: 161-168, 230-230, 238-238, 385-385, 418-418
package.json (1)
75-75: TypeScript version 5.9.3 is valid, current, and has no known vulnerabilities.The verification confirms that 5.9.3 is a legitimate release and is the latest stable version available on npm. No security advisories were found. The dependency in package.json is appropriate and requires no changes.
| expect(lastFrame()).toContain('[D] Copy to Current Directory'); | ||
| expect(lastFrame()).toContain('[E] Edit File'); | ||
| expect(lastFrame()).toContain('[O] Open File'); | ||
| expect(lastFrame()).toContain('[X] Delete File'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add functional tests for the delete action and confirmation flow.
The current tests only verify UI rendering. Essential tests are missing for the core delete functionality, confirmation dialog interaction, and error handling.
Consider adding these test cases:
- Confirmation dialog display test: Verify that pressing 'x' triggers the confirmation dialog
- Successful deletion test: Verify file is deleted after confirming
- Deletion cancellation test: Verify file is NOT deleted after canceling
- Error handling tests: Test permission errors and file-not-found scenarios
- Post-deletion behavior: Verify the menu closes or refreshes after deletion
Example test structure:
test('delete file shows confirmation dialog', async () => {
await withCachedReadOnlyFixture(
{
'test-project': {
'test.md': '# Test',
},
},
async (fixture) => {
const file = createFileInfo(
fixture.path,
'test-project/test.md',
'project-memory',
);
const onClose = vi.fn();
const { stdin, lastFrame } = render(
<MenuActions file={file} onClose={onClose} />,
);
// Trigger delete action
stdin.write('x');
await waitFor(() => {
const output = lastFrame();
if (!output || !output.includes('Delete "test.md"?')) {
throw new Error('Confirmation dialog not shown');
}
}, 100);
// Verify confirmation message
expect(lastFrame()).toContain('Delete "test.md"?');
expect(lastFrame()).toContain('cannot be undone');
},
);
});
test('delete file removes file after confirmation', async () => {
// Test actual file deletion after Y confirmation
});
test('delete file cancels when user declines', async () => {
// Test file is NOT deleted after N/Esc
});Would you like me to generate complete test implementations for the delete functionality?
🤖 Prompt for AI Agents
In src/components/FileList/MenuActions.test.tsx around line 57, the tests only
assert UI rendering and lack functional coverage for the delete action and
confirmation flow; add tests that (1) press 'x' to open the confirmation dialog
and assert lastFrame() contains 'Delete "filename"?' and 'cannot be undone', (2)
simulate confirming deletion (write 'y' or Enter) and verify the file is removed
from the fixture (use withCachedReadOnlyFixture and createFileInfo) and any
expected UI/menu refresh or onClose is called, (3) simulate cancelling deletion
('n' or Escape) and assert the file still exists and menu remains/open state is
unchanged, (4) add error-handling tests by mocking the delete operation to throw
permission or not-found errors and assert appropriate error messages are shown,
and (5) assert post-deletion behavior such as the menu closing or refresh by
checking onClose was called or lastFrame() no longer shows the deleted file; for
all tests use render to obtain stdin/lastFrame, waitFor to poll UI changes, and
inspect the fixture filesystem or mocked delete calls to confirm side effects.
| { | ||
| key: 'x', | ||
| label: 'Delete File', | ||
| description: 'Delete file (requires confirmation)', | ||
| action: async () => { | ||
| const fs = await import('node:fs/promises'); | ||
|
|
||
| // The actual deletion operation | ||
| const performDelete = async () => { | ||
| await fs.unlink(file.path); | ||
| return `✅ File deleted: ${basename(file.path)}`; | ||
| }; | ||
|
|
||
| // Always require confirmation for deletion | ||
| setConfirmMessage( | ||
| `Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`, | ||
| ); | ||
| setIsConfirming(true); | ||
| setPendingAction(() => performDelete); | ||
|
|
||
| // Return early, action will be executed after confirmation | ||
| return ''; | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Critical: Menu remains open after file deletion, showing a non-existent file.
After successfully deleting a file, the menu remains open and continues to display the now-deleted file. This creates a confusing UX where users can attempt further actions on a file that no longer exists, leading to ENOENT errors.
Consider one of these solutions:
Solution 1 (recommended): Close the menu immediately after successful deletion
Modify the performDelete function to trigger menu closure:
const performDelete = async () => {
await fs.unlink(file.path);
+ onClose(); // Close menu immediately after deletion
return `✅ File deleted: ${basename(file.path)}`;
};Solution 2: Add a callback to refresh the parent file list
If the parent component maintains a file list that should be refreshed:
type UseMenuProps = {
readonly file: ClaudeFileInfo;
readonly onClose: () => void;
+ readonly onFileDeleted?: (path: string) => void;
};Then call it after deletion:
const performDelete = async () => {
await fs.unlink(file.path);
+ onFileDeleted?.(file.path);
+ onClose();
return `✅ File deleted: ${basename(file.path)}`;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| key: 'x', | |
| label: 'Delete File', | |
| description: 'Delete file (requires confirmation)', | |
| action: async () => { | |
| const fs = await import('node:fs/promises'); | |
| // The actual deletion operation | |
| const performDelete = async () => { | |
| await fs.unlink(file.path); | |
| return `✅ File deleted: ${basename(file.path)}`; | |
| }; | |
| // Always require confirmation for deletion | |
| setConfirmMessage( | |
| `Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`, | |
| ); | |
| setIsConfirming(true); | |
| setPendingAction(() => performDelete); | |
| // Return early, action will be executed after confirmation | |
| return ''; | |
| }, | |
| }, | |
| { | |
| key: 'x', | |
| label: 'Delete File', | |
| description: 'Delete file (requires confirmation)', | |
| action: async () => { | |
| const fs = await import('node:fs/promises'); | |
| // The actual deletion operation | |
| const performDelete = async () => { | |
| await fs.unlink(file.path); | |
| onClose(); // Close menu immediately after deletion | |
| return `✅ File deleted: ${basename(file.path)}`; | |
| }; | |
| // Always require confirmation for deletion | |
| setConfirmMessage( | |
| `Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`, | |
| ); | |
| setIsConfirming(true); | |
| setPendingAction(() => performDelete); | |
| // Return early, action will be executed after confirmation | |
| return ''; | |
| }, | |
| }, |
Implements file deletion functionality with confirmation dialog.
Users can now delete Claude configuration files directly from the
interactive menu using the 'x' key shortcut.
Changes:
Resolves user request to incorporate file deletion feature
to cleanup redundant tools/agents alongside existing edit/view options.
Summary by CodeRabbit
New Features
Chores